Skip to content

[flow analysis] Clean up promoted types ordering and use of ::#4369

Merged
stereotype441 merged 2 commits into
dart-lang:mainfrom
stereotype441:b_4367_1
May 9, 2025
Merged

[flow analysis] Clean up promoted types ordering and use of ::#4369
stereotype441 merged 2 commits into
dart-lang:mainfrom
stereotype441:b_4367_1

Conversation

@stereotype441
Copy link
Copy Markdown
Member

@stereotype441 stereotype441 commented May 9, 2025

The promotedTypes list in a VariableModel is now consistently ordered from least promoted type to most promoted type, matching the list used in the implementation.

Also, the use of top::rest to refer to a stack is changed to [...rest, top]. This is consistent with the way some stacks are represented in the flow analysis implementation (as a list where pushing and popping happens at the end of the list).

Fixes #4367.

The `promotedTypes` list in a `VariableModel` is now consistently
ordered from least promoted type to most promoted type, matching the
list used in the implementation.

Also, the use of `X::Y` to refer to a stack is adjusted so that `Y`
refers to the top of the stack and `X` refers to the rest of the
stack. This is consistent with the way some stacks are represented in
the flow analysis implementation (as a list where pushing and popping
happens at the end of the list).

Fixes dart-lang#4367.
@stereotype441 stereotype441 requested a review from eernstg May 9, 2025 16:00
Copy link
Copy Markdown
Member

@eernstg eernstg left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, considering that the most important thing is to be consistent.

I would mention, though, that h::t almost inevitably means a list where h is an element which is at the head (first) in the list, and t is the tail (the rest) of the list, and t::h is going to look really weird to a functional programmer.

Perhaps use another notation? One notation that would look normal for functional programmers could be t ++ [h], where ++ is a list concatenation operator. That would allow us to connect to actual Dart code like tail + [head], and it would also handle tail ++ [n1, n0] in a way which is reasonably concise.

@stereotype441
Copy link
Copy Markdown
Member Author

LGTM, considering that the most important thing is to be consistent.

I would mention, though, that h::t almost inevitably means a list where h is an element which is at the head (first) in the list, and t is the tail (the rest) of the list, and t::h is going to look really weird to a functional programmer.

Perhaps use another notation? One notation that would look normal for functional programmers could be t ++ [h], where ++ is a list concatenation operator. That would allow us to connect to actual Dart code like tail + [head], and it would also handle tail ++ [n1, n0] in a way which is reasonably concise.

That's a good point. I've switched to a notation that resembles Dart list syntax: [...tail, head] and [...tail, n1, n0].

I'm going to go ahead and land the change in this form based on your previous lgtm. If you have concerns, let me know and I can always do a follow up PR.

@stereotype441 stereotype441 merged commit b0fa8ca into dart-lang:main May 9, 2025
3 checks passed
@stereotype441 stereotype441 deleted the b_4367_1 branch May 9, 2025 17:38
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

flow-analysis.md treats the order of promotedTypes inconsistently.

2 participants